-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ARROW-5505: [R] namespace cleanup #4491
Conversation
🤔 I understand the issue with masking the base package, but I'm not sure I like
We can totally just not use |
Fair point, I worried about that too. My preference would be to namespace everything (like sparklyr), e.g. |
Yeah, maybe it's worth have a broader discussion about the interface as a whole. These functions
Maybe the R6 classes should be made more user visible and then they would be e.g.
In any case, we should be consistent in what we do, and not just prefixing with |
6ef309b
to
22efa58
Compare
Closing for now; we can continue the discussion on Jira/elsewhere and revisit. |
This is instead of #4491, without the function naming change that we wanted to think about more intentionally. It also removes a few lingering references to `tibble` in the package, which were still passing in tests because tibble is in Suggests and the test hosts install all of the Suggests packages. @romainfrancois Author: Romain Francois <[email protected]> Author: Neal Richardson <[email protected]> Closes #4566 from nealrichardson/clean-imports and squashes the following commits: e0cf005 <Romain Francois> not importing glue::glue 002c0f0 <Romain Francois> no need for glue either at this point 25d4873 <Neal Richardson> Prune unused Imports; fix a couple of lingering tibble references
This patch:
array()
toarrow_array()
andtable()
toarrow_table()
to avoid collision with base R functions of the same name. AFAICT there are no callers out there of those functions (i.e. I checkedsparklyr
and I'm not aware of anything else depending onarrow
in any way).-
from a test file namecrayon
andvctrs
from Imports to Suggests, in response to aR CMD CHECK
note.crayon
was only used in one place in the package and makes sense to be optional;vctrs
is only used in tests.